-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new-changelog to Makefile #8104
Conversation
e6feb43
to
4cf976d
Compare
/kind changelog-not-required |
9328bfe
to
d20f82a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8104 +/- ##
==========================================
+ Coverage 58.77% 59.01% +0.23%
==========================================
Files 358 364 +6
Lines 30070 30270 +200
==========================================
+ Hits 17674 17864 +190
- Misses 10949 10960 +11
+ Partials 1447 1446 -1 ☔ View full report in Codecov by Sentry. |
# make new-changelog CHANGELOG_BODY="Changes you have made" | ||
new-changelog: GH_LOGIN ?= $(shell gh pr view --json author --jq .author.login 2> /dev/null) | ||
new-changelog: GH_PR_NUMBER ?= $(shell gh pr view --json number --jq .number 2> /dev/null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never used gh
cmd, but is this always reliable? Could you explain what does it do? If I have multiple PRs opened and none of them has changelog file, does it get the right PR number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's reliable. It's GitHub's own CLI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gets the right PR number for me.
I could add more vars here for base branch for those who work on other downstream but for folks with their fork and tanzu this should work out of the box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alternative is to curl against GitHub rest API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's based on the current branch you checked out. GitHub will check if the branch is used as a PR against upstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kaovilai we'd need to document somewhere that users need to install the github CLI, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 382 count?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yeah -- that's probably good enough. And this is an optional task that's not called anywhere else, so if it's not there, it won't matter as long as this task isn't executed explicitly.
FYI: I don't really need this any more.. especially if no one else find use of it. |
This PR is valuable to me, but I think we need to document more details on how to use it. In my test, it seems need to create a PR first, then run the added make command.
|
@blackpiglet done. |
Thanks for your effort. |
b6c582f
to
57a0caa
Compare
Signed-off-by: Tiger Kaovilai <[email protected]>
57a0caa
to
0b44777
Compare
Signed-off-by: Tiger Kaovilai [email protected]
Thank you for contributing to Velero!
Please add a summary of your change
Make it easier to create new changelog file correctly.
Example usage:
Does your change fix a particular issue?
Fixes #7865
Please indicate you've done the following:
/kind changelog-not-required
as a comment on this pull request.site/content/docs/main
.